Fix span of unsafe attribute diagnostic #133270
Conversation
|
rustbot has assigned @petrochenkov. Use |
rustbot
added
S-waiting-on-review
| @@ -241,10 +241,6 @@ impl Attribute { | |||
| } | |||
|
|
|||
| impl AttrItem { | |||
| pub fn span(&self) -> Span { | |||
| self.args.span().map_or(self.path.span, |args_span| self.path.span.to(args_span)) | |||
There was a problem hiding this comment.
Are you sure we can't save this by using one of the span ancestor functions like https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/span_encoding/struct.Span.html#method.find_ancestor_in_same_ctxt?
There was a problem hiding this comment.
This is another instance of the span combining problem really.
The x.to(y) operation should just work for things like #[$a = $b] if all metavar spans are recorded.
In fact the parser does exactly the same a.to(b) operation to save the span into the AST, it just may see different spans if nonterminals are involved.
The logic in validate_attrs may work or not work depending on how exactly parts of the attribute was passed (as tt, ident, meta or expr), previously it was fine-tuned for one scenarios, now it's fine tuned for others.
The only certain thing here is that the manual span arithmetic (+/- BytePos(1/2)) should not be performed.
| /// The span of the contents of the attribute. | ||
| /// | ||
| /// This is the span starting from the path and ending at the end of the args. | ||
| pub span: Span, |
There was a problem hiding this comment.
Increasing the size of AttrItem may be a performance sensitive change.
This fixes the span of the
unsafe_attr_outside_unsafediagnostic when the attribute usescfg_attrand comes from a macro. Previously the span it was pointing to was in the wrong place (offset by 2 bytes in the start, and 1 byte in the end), causing a corrupt suggestion.The problem is that the lint was trying to do manual byte manipulation of the
Attributespan to get within the#[and]tokens. However, when the attribute comes fromcfg_attr, that span starts from the attribute path (likeno_mangle), not the#[of thecfg_attr.The solution here is to store the span of the
AttrItemwhile parsing, so that we know for sure that it covers the correct range (the path and all args). We could not useAttrItem::span()(which is removed in this PR), because that function did not correctly account for the path and arguments coming from separate expansion contexts. For example, in the macro expansion of#[$p = $a], the span would be$p =because you are not allowed to generate a span across expansion contexts.Fixes #132908